-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat: S3 파일 업로드 베이스 코드 구현 #24
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review by ChatGPT
src/main/java/com/seveneleven/devlens/global/exception/BusinessException.java
Show resolved
Hide resolved
src/main/java/com/seveneleven/devlens/global/exception/GlobalExceptionHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/com/seveneleven/devlens/global/util/file/dto/FileMetadataDto.java
Show resolved
Hide resolved
src/main/java/com/seveneleven/devlens/global/util/file/entity/FileMetadata.java
Show resolved
Hide resolved
src/main/java/com/seveneleven/devlens/global/util/file/entity/FileMetadataHistory.java
Show resolved
Hide resolved
src/main/java/com/seveneleven/devlens/global/util/file/repository/FileMetadataRepository.java
Show resolved
Hide resolved
src/main/java/com/seveneleven/devlens/global/util/file/FileValidator.java
Outdated
Show resolved
Hide resolved
src/main/java/com/seveneleven/devlens/global/util/file/FileValidator.java
Outdated
Show resolved
Hide resolved
src/main/java/com/seveneleven/devlens/global/util/file/FileValidator.java
Outdated
Show resolved
Hide resolved
FileMetadata metadata = FileMetadata.builder() | ||
.fileCategory(fileCategory) | ||
.fileDisplayTitle(file.getOriginalFilename()) | ||
.fileTitle(uniqueFileName) | ||
.writtenBy(uploaderId) //TODO) Audit | ||
.writtenAt(LocalDateTime.now()) //TODO) Audit | ||
.contentType(file.getContentType()) | ||
.fileFormat(file.getOriginalFilename().substring(originalFilename.lastIndexOf('.') + 1)) | ||
.fileSize(file.getSize() / 1024.0) // KB | ||
.filePath(filePath) | ||
.referenceId(referenceId) // 필요 시 설정 | ||
.createdBy(1L) //TODO) Audit | ||
.createdAt(LocalDateTime.now()) //TODO) Audit | ||
.build(); | ||
|
||
//FileMetaData 저장 | ||
FileMetadata savedMetadata = fileMetadataRepository.save(metadata); | ||
|
||
//DTO로 변환 후 반환 | ||
return FileMetadataDto.toDto(savedMetadata); | ||
|
||
} catch (Exception e){ | ||
//저장 실패시 S3에서 삭제 | ||
s3ClientService.deleteFile(s3Key); | ||
|
||
throw new BusinessException(e.getMessage(), ErrorCode.S3_UPLOAD_FAIL_ERROR); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 이 메소드에서는 "S3에 파일 저장" 과 "엔티티 생성", "DB에 엔티티 저장" 이렇게 3가지 일을 하고 있습니다.
엔티티 생성과 DB에 저장하는 것은 크게 한가지 책임으로 볼 수 있지만 S3에 파일을 저장하는건 다른 책임이라고 생각됩니다.
클래스 이름이 FileService
인 만큼 DB에 저장하는 로직은 다른 서비스로 분리 하는것을 추천 드립니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기능 구현 후 리팩토링 과정에서 기능별로 분리하겠습니다.
src/main/java/com/seveneleven/devlens/global/util/file/Service/S3ClientService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/seveneleven/devlens/global/util/file/controller/FileController.java
Outdated
Show resolved
Hide resolved
return new StringBuilder() | ||
.append(uploaderId) | ||
.append("/") | ||
.append(category) | ||
.append("/") | ||
.append(fileName) | ||
.toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 코드를 잘 못 이해한 것 같습니다. 지금 상황이라면 StringJoiner를 사용하는게 더 효과적인것 같습니다.
지금 상황으로도 딱히 문제는 없으므로 참고만 해주시면 좋을 것 같아요.
바로 MERGE하지 마세요! 검토 후 MERGE해야합니다!
closes : #23
✅ 최근 작업 주제 (하나 이상의 주제를 선택해주세요.)
🏆 구현 목표
S3 파일 업로드 기본 세팅 및 베이스 코드 구현
📋 구현 사항 설명 (작업한 내용을 상세하게 기록합니다.)
🔍 테스트 방법 (변경 사항을 확인하기 위한 테스트 방법을 기술합니다.)
🛠️ 쓰이는 모듈
💬 기타 질문 및 특이 사항